Fix gateway launchd log growth#921
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces improved logging controls for the gateway, including suppressing console logging by default, dynamically setting logger propagation based on environment variables, and reducing WeChat API polling log noise by logging updates at the debug level. It also adds a startup script with log rotation capabilities and corresponding tests. The review feedback suggests simplifying the platform-specific file size check in the shell script, moving the console logging suppression inside the try block to guarantee environment variable restoration, and adding a missing assertion in the logging configuration tests.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| file_size_bytes() { | ||
| local file="$1" | ||
| if stat -f %z "$file" >/dev/null 2>&1; then | ||
| stat -f %z "$file" | ||
| return | ||
| fi | ||
| stat -c %s "$file" | ||
| } |
There was a problem hiding this comment.
The file_size_bytes function can be simplified and optimized to avoid running stat twice on macOS/BSD systems. By using the || operator, we can attempt the macOS/BSD-specific stat command and fall back to the Linux-specific one in a single line, which is also fully compatible with set -e.
| file_size_bytes() { | |
| local file="$1" | |
| if stat -f %z "$file" >/dev/null 2>&1; then | |
| stat -f %z "$file" | |
| return | |
| fi | |
| stat -c %s "$file" | |
| } | |
| file_size_bytes() { | |
| local file="$1" | |
| stat -f %z "$file" 2>/dev/null || stat -c %s "$file" | |
| } |
| previous_disable_console_log = os.environ.get("AWORLD_DISABLE_CONSOLE_LOG") | ||
| previous_gateway_console_log = os.environ.get(GATEWAY_CONSOLE_LOG_ENV) | ||
| gateway_log_path: Path | None = None | ||
| _suppress_gateway_console_logging() | ||
| try: | ||
| await server.serve() | ||
| gateway_log_path = _configure_gateway_file_logging(base_dir=resolved_base_dir) |
There was a problem hiding this comment.
To ensure robust environment variable restoration, _suppress_gateway_console_logging() should be called inside the try block. If an exception (such as a KeyboardInterrupt or other asynchronous exception) occurs after _suppress_gateway_console_logging() is called but before entering the try block, the environment variables will not be restored in the finally block.
| previous_disable_console_log = os.environ.get("AWORLD_DISABLE_CONSOLE_LOG") | |
| previous_gateway_console_log = os.environ.get(GATEWAY_CONSOLE_LOG_ENV) | |
| gateway_log_path: Path | None = None | |
| _suppress_gateway_console_logging() | |
| try: | |
| await server.serve() | |
| gateway_log_path = _configure_gateway_file_logging(base_dir=resolved_base_dir) | |
| previous_disable_console_log = os.environ.get("AWORLD_DISABLE_CONSOLE_LOG") | |
| previous_gateway_console_log = os.environ.get(GATEWAY_CONSOLE_LOG_ENV) | |
| gateway_log_path: Path | None = None | |
| try: | |
| _suppress_gateway_console_logging() | |
| gateway_log_path = _configure_gateway_file_logging(base_dir=resolved_base_dir) |
| monkeypatch.setenv("AWORLD_GATEWAY_CONSOLE_LOG", "true") | ||
| gateway_cli._configure_gateway_file_logging(base_dir=tmp_path) |
There was a problem hiding this comment.
The test test_configure_gateway_file_logging_can_suppress_console_propagation sets AWORLD_GATEWAY_CONSOLE_LOG to "true" and calls _configure_gateway_file_logging, but it misses the assertion to verify that propagate is indeed set to True.
| monkeypatch.setenv("AWORLD_GATEWAY_CONSOLE_LOG", "true") | |
| gateway_cli._configure_gateway_file_logging(base_dir=tmp_path) | |
| monkeypatch.setenv("AWORLD_GATEWAY_CONSOLE_LOG", "true") | |
| gateway_cli._configure_gateway_file_logging(base_dir=tmp_path) | |
| assert logging.getLogger("aworld.gateway").propagate is True |
|
Closing this PR because it included scripts/run-aworld-gateway.sh. Replacing it with a clean branch that removes that file from the PR diff. |
1 participant
Summary
Test Plan